Skip to content

Conversation

@rhshadrach
Copy link
Member

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

@rhshadrach rhshadrach added Testing pandas testing functions or related to the test suite Visualization plotting labels Oct 7, 2023

# yerr is iterator
ax = _check_plot_works(df.plot, yerr=itertools.repeat(0.1, len(df)))
ax = _check_plot_works(df.plot, yerr=np.full(len(df), 0.1))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docstring for yerr says array-like; I don't think we should expect an iterator to work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test name and comment a line up specifically say iterator. i dont care about iterators either, but someone at some point did this intentionally. may merit a whatsnew if we are officially giving up on this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - I took another look. This goes back to when the error bars were added in #6834. The issue is actually on our end - we pass kwargs twice to two different subplots and on the 2nd the generator is exhausted. We can fix by making a copy in the test.

It seems to me this could break in the future because the matplotlib docs don't say generators are supported, but we can leave as-is for now.

tlabels = axis.get_ticklabels()
for loc, label in zip(tlocs, tlabels):
xp = conv._from_ordinal(loc).strftime("%H:%M:%S.%f")
xp = conv._from_ordinal(loc).strftime("%H:%M:%S")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed the decimal here is not shown in the label when plotting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question. On matplotlib 3.5, the code rs = str(label.get_text()) returns the empty string and the subsequent assert is not ran. On 3.6, we get "21:59:51" for rs. In both cases, 21:59:51 is displayed on the resulting plot. So I believe the test was incorrect on our end.

for line in xaxis.get_ticklabels():
if len(line.get_text()) > 0:
assert line.get_rotation() == 30
assert line.get_rotation() == 0.0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The label is not rotated visually.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same, should it be?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the previous - on 3.5 line.get_text() is returning the empty string. In both 3.5 and 3.6, line.get_rotation() reports 0.0, agreeing with the resulting plot.

@rhshadrach rhshadrach marked this pull request as draft October 8, 2023 20:01
@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Nov 14, 2023
@mroeschke
Copy link
Member

Still planning on working on this?

@rhshadrach
Copy link
Member Author

Yes - I would like to get back to this, but not in the next few weeks at least. Taking it off the queue.

@rhshadrach rhshadrach closed this Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stale Testing pandas testing functions or related to the test suite Visualization plotting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants